Mint for me - profile wallet selection#1925
Conversation
Signed-off-by: prxt6529 <prxt@6529.io>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds dev-mode wallet impersonation to connect context, extends RecipientSelector with profile/wallet rendering flags, implements dual minting flows (mint-for-fren vs mint-for-me) in ManifoldMintingConnect, adds mint diagnostics UI in ManifoldMintingWidget, and introduces tests for wallet/profile switching during mint flows. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Connect as SeizeConnectContext
participant Mint as ManifoldMintingConnect
participant Selector as RecipientSelector
participant Callback as onMintFor
User->>Connect: (optionally) enable DEV impersonation / connect wallet
Connect-->>Mint: provide `address`, `isConnected`, `isAuthenticated`
User->>Mint: Toggle "Mint for Fren" / "Mint for Me"
alt Mint for Fren
Mint->>Selector: render Fren profile & wallets
User->>Selector: select Fren profile / wallet
Selector-->>Mint: update Fren wallet state
Mint->>Callback: invoke onMintFor(Fren wallet)
else Mint for Me
Mint->>Mint: reset Fren state, derive connected recipient
Mint->>Selector: render connected profile (profile-change disabled)
User->>Selector: select connected wallet (if multiple)
Selector-->>Mint: update selected connected wallet
Mint->>Callback: invoke onMintFor(connected wallet)
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@components/auth/SeizeConnectContext.tsx`:
- Around line 368-377: The code allows client-side impersonation because PUBLIC
runtime vars (USE_DEV_AUTH, DEV_MODE_WALLET_ADDRESS, DEV_MODE_AUTH_JWT) are
baked into the bundle and only gated by getNodeEnv()/isDevLikeEnv in the
impersonatedAddress logic; to fix, either remove/omit those three keys from the
public runtime env at build-time so they never reach the client, or add an
additional deployment-safe guard (e.g., require publicEnv.DEPLOY_ENV ===
'production' or process.env.DEPLOY_ENV === 'production') in the impersonation
check before using USE_DEV_AUTH/DEV_MODE_WALLET_ADDRESS in the
impersonatedAddress computation to ensure impersonation cannot activate due to
NODE_ENV misconfiguration.
In `@components/manifoldMinting/ManifoldMintingConnect.tsx`:
- Around line 72-86: Destructure onMintFor from props and use it instead of
props.onMintFor inside the useEffect, remove props from the dependency array and
include onMintFor (i.e., useEffect deps: [selectedFrenWallet,
selectedMintForMeWallet, mintForFren, account.address, onMintFor]); also remove
the unsafe "as string" cast and explicitly guard that account.address exists
(e.g., if neither selectedMintForMeWallet nor account.address is defined, bail
out) before calling onMintFor with selectedMintForMeWallet ?? account.address,
keeping references to useEffect, selectedFrenWallet, selectedMintForMeWallet,
mintForFren, account.address, and onMintFor.
🧹 Nitpick comments (2)
components/manifoldMinting/ManifoldMintingConnect.tsx (2)
60-63:resetFrenis missing from theuseEffectdependency array.The exhaustive-deps rule would flag this. While functionally correct (the state setters inside
resetFrenare stable), wrappingresetFreninuseCallbackand adding it to the deps list would be the idiomatic approach.Proposed fix
- function resetFren() { - setSelectedFrenProfile(null); - setSelectedFrenWallet(null); - } + const resetFren = useCallback(() => { + setSelectedFrenProfile(null); + setSelectedFrenWallet(null); + }, []); useEffect(() => { resetFren(); setMintForFren(false); - }, [account.address]); + }, [account.address, resetFren]);
104-127:noopProfileSelectionis recreated on every render — consider hoisting or memoizing.
noopProfileSelectionis defined insideprintMintForMe, which is called during render. SinceRecipientSelectorreceives a new function reference each time, this could trigger unnecessary re-renders of that component. A module-level no-op would be simpler.Proposed fix — hoist the no-op
At the top of the file (or outside the component):
+const noopProfileSelection = (_: CommunityMemberMinimal | null) => undefined; + export default function ManifoldMintingConnect(Then remove the local declaration inside
printMintForMe.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@components/manifoldMinting/ManifoldMintingConnect.tsx`:
- Around line 75-93: When mintForFren is true but selectedFrenWallet is empty
the current useEffect calls onMintFor with the user's address; change the
behavior so the effect does not call onMintFor in that case (or explicitly call
onMintFor(null) if you want to signal "no selection") so the parent can disable
the mint action until a fren is chosen. Update the useEffect logic around the
mintForFren / selectedFrenWallet check in ManifoldMintingConnect (the block
referencing mintForFren, selectedFrenWallet, selectedMintForMeWallet,
account.address, and onMintFor) to return early without invoking onMintFor when
mintForFren is true and selectedFrenWallet is falsy.
🧹 Nitpick comments (2)
components/auth/SeizeConnectContext.tsx (2)
368-384: Consider memoizing the impersonation derivation.
getNodeEnv(), the localhost check, andgetAddress()are called on every render. While the cost is trivial, wrapping the block in auseMemo(with an empty dep array, since the inputs are static) would make the intent explicit and prevent unnecessary calls.♻️ Suggested refactor
- const nodeEnv = getNodeEnv(); - const isDevLikeEnv = - nodeEnv === "development" || nodeEnv === "test" || nodeEnv === "local"; - const isLocalHost = - typeof window !== "undefined" && - (window.location.hostname === "localhost" || - window.location.hostname === "127.0.0.1" || - window.location.hostname === "::1" || - window.location.hostname.endsWith(".local")); - const impersonatedAddress = - isDevLikeEnv && - isLocalHost && - publicEnv.USE_DEV_AUTH === "true" && - publicEnv.DEV_MODE_WALLET_ADDRESS && - isAddress(publicEnv.DEV_MODE_WALLET_ADDRESS) - ? getAddress(publicEnv.DEV_MODE_WALLET_ADDRESS) - : undefined; + const impersonatedAddress = useMemo(() => { + const nodeEnv = getNodeEnv(); + const isDevLikeEnv = + nodeEnv === "development" || nodeEnv === "test" || nodeEnv === "local"; + const isLocalHost = + typeof window !== "undefined" && + (window.location.hostname === "localhost" || + window.location.hostname === "127.0.0.1" || + window.location.hostname === "::1" || + window.location.hostname.endsWith(".local")); + return isDevLikeEnv && + isLocalHost && + publicEnv.USE_DEV_AUTH === "true" && + publicEnv.DEV_MODE_WALLET_ADDRESS && + isAddress(publicEnv.DEV_MODE_WALLET_ADDRESS) + ? getAddress(publicEnv.DEV_MODE_WALLET_ADDRESS) + : undefined; + }, []);
614-631: Wallet metadata is absent during impersonation — may confuse dev-mode consumers.When
impersonatedAddressis active,walletName,walletIcon, andisSafeWalletwill all be undefined/false since no real wallet is connected. If any downstream component (e.g., wallet display UI) renders based onisConnected === truebut also readswalletName, it may show a blank or broken state in dev mode.Not a production concern, but worth a brief note or hardcoded fallback (e.g.,
walletName: impersonatedAddress ? "Dev Wallet" : walletInfo?.name) for a smoother dev experience.
Signed-off-by: prxt6529 <prxt@6529.io>
|



Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores